fix: recover interrupted paykit sessions by ben-kaufman · Pull Request #1339 · synonymdev/bitkit-android

ben-kaufman · 2026-09-24T19:45:15Z

A failed Paykit session restore after connection loss or a device clock change could make an existing profile appear missing and discard contacts. This PR preserves saved identity data, retries recovery when connectivity returns or the app resumes, and corrects retry and billing-reminder timing.

Fixes #1344.

Counterpart: iOS PR.

Related: #1334 also changes the persisted-identity lookup as part of backup protection; that overlapping hunk needs reconciling when both PRs merge.

Description

Automatically retries saved-session restoration on reconnect and foreground from any screen, coalescing concurrent attempts. Recovery cannot overwrite an active authorization or reinstate a session after teardown.
Reads the saved identity without restoring its old grant, and preserves unreadable SDK state so a failed reconnect cannot treat existing contacts as a fresh identity.
Retains cached profile/contact overrides after restoration fails and limits failed Ring-auth cleanup to a newly installed session, preserving the previous or unreadable session.
Keeps Pubky signup disabled while an existing identity is saved, including when its credentials are temporarily unreadable.
Clears cached profile/contact overrides on a confirmed identity change while preserving same-identity recovery and legacy backup caches.
Resumes identity publication after a backward clock correction instead of waiting for an old future timestamp.
Rechecks the billing date when a subscription notification worker runs, retrying if the period is still in the future.

Out of Scope

Pubky grant and payment-expiry validation: real expiration and authorization checks remain enforced; no backend clock-skew bypass.
Notification delivery precision: WorkManager remains best effort and can delay reminders through OS scheduling or retry backoff.
Wallet backup/restore protection: tracked separately in fix: preserve paykit payment state #1334. The reported missing Bitcoin wallet remains unconfirmed and is not claimed fixed here.

Design

N/A — no UI changes.

Preview

Android offline-start/reconnect recordings were captured locally: the original build lost the profile name and did not recover; the fixed build retained the name and contact and restored the same identity about 1.7 seconds after reconnecting. Live clock-change recording remains pending.

QA Notes

Journeys

N/A — not drivable; see Manual Tests.

Manual Tests

Live grant-session clock-change E2E has not been rerun. Android offline cold-start/reconnect and contact preservation were verified on a disposable regtest emulator. The full procedure is in paykit-clock-changes.md.

regression: with a saved profile/contact and valid local or Ring session, cold-start offline, then reconnect or foreground from the contact/profile screen → cached data stays present and the same identity recovers without signing out or reauthorization; an expired/revoked grant still needs authorization — network fault injection not in Capabilities.
regression: start Ring authorization, sign out or reset while recovery is pending → automatic recovery cannot replace the authorization or resurrect deleted credentials — network fault injection not in Capabilities.
regression: use fresh test wallets with a saved contact, covering local-secret and Ring sessions → move the device clock a month ahead, attempt recovery, correct it and retry; repeat with a backward change → saved contacts/profile remain available and private payments recover, with reauthorization if the grant expired — isolated device-clock control not in Capabilities.
regression: after failed restoration, open the profile button and reauthorize with Ring without signing out → the same identity keeps its cache; a different identity cannot display the old name/avatar or contact labels even when its profile is unavailable — device-clock fault injection not in Capabilities.
regression: change only the timezone between America/New_York, Pacific/Kiritimati and Pacific/Pago_Pago, including a daylight-saving boundary → authentication and UTC billing boundaries stay unchanged — OS timezone and date control not in Capabilities.
regression: schedule a subscription reminder and defer an unavailable payment request, then change the clock → no payment-due reminder before its billing boundary, and retries recover after time is corrected — device-clock fault injection not in Capabilities.

Automated Checks

updated PaykitSdkServiceTest.kt — identity lookup failures stop activation without deleting saved state or credentials; activation tests cover same/different owners, normalized keys, legacy backup caches, and cache-reset failure.
updated PubkyAuthHandlerRegistrarTest.kt and AppViewModelSendFlowTest.kt — saved/unreadable identities do not advertise signup; reconnect and foreground trigger recovery.
updated PubkyRepoTest.kt — automatic retry after failure, unreadable credentials, queued wipe, active Ring authorization, and cancelled completion racing retry; restoration preserves profile data for retry; failed Ring auth cleans up only a session installed by that attempt, including unreadable ownership and cancellation coverage; identity switches discard stale in-memory profile/contact data and reset the contact-load marker.
updated PubkyIdentityRepublishTest.kt — clock rollback retries publication and then resumes throttling.
updated PaykitSubscriptionNotificationSchedulerTest.kt — a worker running before the billing boundary defers its reminder.
updated PaykitSubscriptionTest.kt — timezone and DST changes preserve UTC billing boundaries.
ran local Gradle compile, full unit tests and Detekt with a command-scoped init script excluding stale Maven Local artifacts — all 2,911 tests passed across 192 suites, with no failures or skips. Detekt has no findings in changed files and 15 existing findings in untouched files. Final cleanup also passed all 110 PubkyRepo tests, compile and APK build. No host or funded test-device clock was changed.

github-actions · 2026-09-24T19:53:04Z

Regtest APK

Built from 77239d2 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

greptile-apps · 2026-09-24T19:54:29Z

Confidence Score: 4/5

The PR should not merge until preserved Pubky cache data is isolated when recovery proceeds with a different identity.

Findings

Cached identity data crosses accounts ▶

Summary

The PR preserves Paykit session and cached Pubky data across failed recovery, retries identity publication after clock rollback, and prevents subscription reminders from posting before their billing boundary.

Adds recovery, timing, and timezone tests plus a manual clock-change journey.
The preserved Pubky cache needs an identity boundary before a different account can authenticate.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Identity A restoration fails] --> B[Global Pubky cache retained]
  B --> C[Identity B authenticates through Ring]
  C --> D[B contacts loaded]
  B --> D
  D --> E[A contact overrides may affect B]

_{Reviews (1) · Last reviewed commit: "fix: recover paykit after clock changes"}

jvsena42

One medium finding, gated behind the Paykit UI flag, so it affects opted-in users on released builds and does not block. There is also a low one. Both are inline. The toast finding also applies to synonymdev/bitkit-ios#796 (AppScene toasts on every sessionRestorationFailed change).

Checked and clean:

Billing reminders only post a notification; the tap path never pays. A retry only happens when nothing was posted, so no duplicate reminders. A worker that returns retry is still an upcoming period, so synchronize() keeps its work name rather than cancelling it.
Sign-out and wipe take initializeMutex, so a queued retry then sees no identity. After a failed sign-out, _publicKey stays set and retry does nothing.
Ring cancel: before approval, cancelling ends the attempt without holding the lock. After approval, the new session is revoked and the keychain cleared, so retry finds nothing. During Authenticating, retry is guarded.
Recovery cannot overwrite a live authorization: _authState != Idle, and completeAuthentication holds the mutex from approval through activation.
No nested initializeMutex in deleteProfile, discardAbandonedSession, approveSignupAuth or restoreSessionBackupState.
No secrets logged; new logs use redacted().
The republish clock-rollback check matches iOS.
Signup alias stays disabled on an unreadable keychain (getOrDefault(true)).

piotr-iohk

QA reviewed on 0d32249.

QA review

Reviewed the full PR diff against its merge base, at 0d32249.

No new actionable code findings.

Checked saved-session retry on reconnect and resume, silent restoration failures, cache isolation when the saved owner changes, signup suppression while a saved identity is unreadable, identity republish after a clock rollback, and billing reminders that run before the UTC period start. The earlier comments on cached identity data, repeated session-expired toasts, and wipe waiting on profile loading match the current source.

Unit and scheduler tests were inspected, not executed in this review. The author reports the local unit suite passed. CI on this commit succeeded, including the local end-to-end jobs. Those jobs do not drive device-clock or network-fault recovery. The manual procedure in journeys/paykit-clock-changes.md is the runtime check. Shared recovery, cache, and billing behavior was compared with bitkit-ios#796 (003030e); that was not a full review of the iOS pull request.

Device testing: not performed in this review.

Ready for device testing.

ovi-reviewer

Advice: ✅ Approve

Review: diff 16 files.
Matches synonymdev/bitkit-ios#796.

Findings:
3 inline (non-blocking)

Audit:
Audited - no findings.

Coverage:
QA: waits for the other reviewers' approval, or @ovi-reviewer test

Reviewed by gpt-6-sol-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner) · wrong <why> (owner)

ovi-reviewer

Advice: ✅ Approve

Reaudit: diff 4 files.
No new findings; the rest is in the review.

Reviewed by gpt-6-sol-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner) · wrong <why> (owner)

jvsena42

553e27b: ovi-reviewer's three threads are fixed. The unlock and the flag reset are consecutive non-suspending statements, so finally cannot unlock twice. Both loaders drop stale results via _publicKey, and the new wipe-during-loads test covers the main race. One low-severity follow-up inline.

piotr-iohk

QA reviewed on 553e27b.

QA review

1 actionable finding — resolve or provide an evidence-backed rebuttal.

Reviewed the full PR diff against its merge base bde47ec, at 553e27b.

Checked saved-session retry on reconnect and foreground, silent restoration retries, cache isolation when the saved owner changes, identity republish after a clock rollback, and the billing-worker time gate. Shared recovery, cache, republish, and billing behavior was compared with bitkit-ios#796 (003030e).

1 actionable finding, same mechanism as the open thread on superseded profile and contact loads.

Unit and scheduler tests were inspected, not executed. The author reports the local unit suite passed. CI, lint, and local end-to-end succeeded on this commit. Those jobs do not drive device-clock or network-fault recovery. The manual procedure in journeys/paykit-clock-changes.md is the runtime check.

Device testing: not performed in this review.

Additional test cases

Android: Authorize identity A and delay its profile and contact fetches. Sign out, then authorize identity B before those fetches finish. B's name and contacts should appear without opening Profile or Contacts again, and A's name, avatar, and contact labels should not.

Findings

[LOW] Reload profile after a superseded fetch — inline at app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:381.

ovi-reviewer

Advice: ✅ Approve

Reaudit: diff 2 files.
No new findings; the rest is in the review.

Reviewed by gpt-6-sol-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner) · wrong <why> (owner)

piotr-iohk

QA reviewed on 5600005.

QA review

No new actionable code findings.

Reviewed the full PR diff against its merge base bde47ec, at 56000054.

Checked saved-session retry on reconnect and resume, silent restoration after the startup failure is reported, cache isolation when the saved owner changes, signup suppression while a saved identity is unreadable, identity republish after a clock rollback, the billing-worker time gate, and profile/contact loads after an older fetch is still in flight. At this revision those loads wait for the in-flight fetch and then load the current identity. That matches the behavior requested on the open thread, and completed authentication loads new identity after previous loads finish in PubkyRepoTest.kt covers the handoff. Shared recovery, cache, republish, and billing behavior was compared with bitkit-ios#796 (003030e).

Unit and scheduler tests were inspected. This review did not execute them. Lint passed on this commit. The debug build was still running. Those jobs do not drive device-clock or network-fault recovery. The manual procedure in journeys/paykit-clock-changes.md is the runtime check.

Device testing: not performed in this review.

Additional test cases

Android: Cold-start online with an expired or revoked Ring grant. The session-expired notice appears once. Background the app, reopen it, and reconnect the network. The notice stays absent, the cached name remains, and the profile header still opens Ring authorization for that same identity.

ovitrif · 2026-09-28T20:18:36Z

@ben-kaufman this conflicts with master since the Ring PRs merged. Can you resolve it?

ovi-reviewer

Advice: ✅ Approve

Reaudit: no code change since 7a1573d, 0 files.
No new findings; the rest is in the review.
Dropped as declined: A legacy cache can appear under another Ring identity when both owners are unreadable; When A’s saved session cannot be restored and the SDK has no readable owner, this null check keeps A’s….
Pair PR synonymdev/bitkit-ios#796: not compared; the Android ownerless persistent-cache migration is enough to decide this change.

Reviewed by autopilot-verdict via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner) · wrong <why> (owner)

addressed - reaudit confirmed

jvsena42

Re-checked 5600005 and 7a1573d. One LOW left, as a reply on the existing lock() thread: createIdentity still runs the loads under initializeMutex, which now blocks behind an in-flight load. Paykit-gated; affects opted-in users on released builds once the flag is on.

Clean: the lock() + stale-pk re-check unlocks on mismatch before any suspension, so a cancelled waiter never holds the lock; no lock-order inversion (nothing holds the load mutexes while waiting on initializeMutex). The cache owner tag comes from the same SDK-derived key as _publicKey, and matches() normalizes the pubky prefix, so same-identity restores keep the cache. activateBootstrapResult resets on a confirmed identity change from either owner. Backup restore resets the store before signIn. Upgrade from v2.5.0 pubky.json: ownerPublicKey defaults to null with ignoreUnknownKeys, the first load tags it, and downgrade still decodes.

ovi-reviewer

Advice: ✅ Approve

Reaudit: diff 2 files.
No new findings; the rest is in the review.
Equivalent pair: synonymdev/bitkit-ios#796.

Reviewed by gpt-6-sol-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner) · wrong <why> (owner)

piotr-iohk

QA review

Scope: Full reassessment of the complete PR diff against merge base 3bbcb93, at 77239d2, after the comparison base moved on from the earlier review of 56000054.

No new actionable code findings.

Saved-session retry on reconnect and resume keeps the cached profile, reports the session-expired toast only for the startup failure, and stays behind an in-progress Ring adoption or wipe. Signup stays disabled while a saved or unreadable identity exists. A confirmed identity change clears the cached name and contact overrides. An ownerless legacy cache remains when the SDK owner is also unreadable, matching the resolved discussion on PaykitSdkService.kt. Identity republish runs again after a backward clock jump, and the billing worker defers a reminder that starts before the billing instant. Profile and contact loads wait for an older in-flight fetch, then drop a stale result. createIdentity loads after releasing initializeMutex, which covers the remaining change request on 7a1573d.

Validation: inspected the updated unit tests and did not execute them in this pass. Build, lint, and detekt passed on this commit. The local Appium shards do not drive device-clock or connectivity fault injection; that coverage stays on the manual procedure already in the PR. The iOS counterpart was outside this pass.

Device testing: not performed in this review.

Ready for device testing.

jvsena42

Re-checked 77239d2. No findings; the createIdentity lock thread is resolved.

ovi-reviewer

Advice: ✅ Approve

Reaudit: diff 2 files.
No new findings; see the review.
Matching PR: synonymdev/bitkit-ios#796.

QA:

Test 1 ⚠️ not verified: Network faults could not be tested.

Test 2 ⚠️ not verified: Network faults could not be tested.

Test 3 ⚠️ not verified: Device clock changes could not be tested.

Test 4 ⚠️ not verified: Device clock changes could not be tested.

Test 5 ⚠️ not verified: OS timezone changes could not be tested.

Test 6 ⚠️ not verified: Device clock changes could not be tested.

Warning

Tests 1–6 remain unverified because their network fault, device clock, or OS timezone behavior could not be tested.

Coverage:
Unit tests: 80% - New tests cover cache failure and wiping during a paused contact load; identity checks and the ViewModel caller were reviewed.

Reviewed by gpt-6-sol-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner) · wrong <why> (owner)

addressed according to history

fix: recover paykit after clock changes

f21f86e

ben-kaufman mentioned this pull request Sep 24, 2026

fix: recover interrupted paykit sessions synonymdev/bitkit-ios#796

Open

6 tasks

ovitrif added this to the 2.6.0 milestone Sep 24, 2026

greptile-apps Bot reviewed Sep 24, 2026

View reviewed changes

Comment thread app/src/main/java/to/bitkit/repositories/PubkyRepo.kt Outdated

ben-kaufman added 2 commits September 24, 2026 23:08

fix: isolate pubky cache when identity changes

9890462

fix: retry saved paykit sessions after connection loss

8a1a7cb

ben-kaufman changed the title ~~fix: recover paykit after clock changes~~ fix: recover interrupted paykit sessions Sep 25, 2026

ben-kaufman mentioned this pull request Sep 25, 2026

[Bug]: Pubky profile falls back to "Your Name" after Shop Paykit setup #1344

Closed

ovitrif removed this from the 2.6.0 milestone Sep 25, 2026

ben-kaufman added 2 commits September 25, 2026 16:12

fix: preserve restored paykit identity

d93a2cb

Merge master into fix/paykit-clock-recovery

5bc92b8

jvsena42 self-requested a review September 25, 2026 16:20

jvsena42 reviewed Sep 25, 2026

View reviewed changes

Comment thread app/src/main/java/to/bitkit/repositories/PubkyRepo.kt Outdated

Comment thread app/src/main/java/to/bitkit/repositories/PubkyRepo.kt Outdated

fix: keep paykit retry failures silent

6e37e65

ben-kaufman requested a review from jvsena42 September 25, 2026 17:27

fix: preserve paykit restoration failure signal

0d32249

piotr-iohk reviewed Sep 28, 2026

View reviewed changes

ovitrif self-requested a review September 28, 2026 13:56

ovi-reviewer Bot reviewed Sep 28, 2026

View reviewed changes

Comment thread app/src/main/java/to/bitkit/repositories/PubkyRepo.kt Outdated

Comment thread app/src/main/java/to/bitkit/repositories/PaykitSubscriptionNotificationScheduler.kt Outdated

Comment thread app/src/main/java/to/bitkit/services/PaykitSdkService.kt Outdated

fix: release paykit auth lock before loading

553e27b

ovitrif mentioned this pull request Sep 28, 2026

feat: enable paykit by default synonymdev/bitkit-ios#818

Merged

3 tasks

ovi-reviewer Bot reviewed Sep 28, 2026

View reviewed changes

jvsena42 mentioned this pull request Sep 28, 2026

feat: enable paykit by default #1359

Merged

3 tasks

jvsena42 requested changes Sep 28, 2026

View reviewed changes

Comment thread app/src/main/java/to/bitkit/repositories/PubkyRepo.kt Outdated

piotr-iohk reviewed Sep 28, 2026

View reviewed changes

Comment thread app/src/main/java/to/bitkit/repositories/PubkyRepo.kt Outdated

fix: reload paykit identity after stale fetch

5600005

ovi-reviewer Bot reviewed Sep 28, 2026

View reviewed changes

ben-kaufman requested a review from jvsena42 September 28, 2026 17:35

ben-kaufman requested a review from piotr-iohk September 28, 2026 17:35

piotr-iohk reviewed Sep 28, 2026

View reviewed changes

ovitrif mentioned this pull request Sep 28, 2026

Upgrade Pubky to 0.12 and Pubky Noise to rc10 pubky/paykit-rs#165

Merged

Merge master into fix/paykit-clock-recovery

6fa5e7f

This comment was marked as resolved.

Sign in to view

fix: isolate pubky cache by identity

7a1573d

This comment was marked as resolved.

Sign in to view

ovi-reviewer Bot reviewed Sep 29, 2026

View reviewed changes

jvsena42 previously requested changes Sep 29, 2026

View reviewed changes

fix: release pubky creation lock before loading

77239d2

ben-kaufman requested a review from jvsena42 September 29, 2026 15:57

ovi-reviewer Bot reviewed Sep 29, 2026

View reviewed changes

piotr-iohk reviewed Sep 29, 2026

View reviewed changes

jvsena42 reviewed Sep 29, 2026

View reviewed changes

ovi-reviewer Bot reviewed Sep 29, 2026

View reviewed changes

ovitrif approved these changes Sep 29, 2026

View reviewed changes

ovitrif merged commit ea38428 into master Sep 29, 2026
21 checks passed

ovitrif deleted the fix/paykit-clock-recovery branch September 29, 2026 20:10

Conversation

ben-kaufman commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Out of Scope

Design

Preview

QA Notes

Journeys

Manual Tests

Automated Checks

Uh oh!

github-actions Bot commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Regtest APK

Uh oh!

greptile-apps Bot commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Diagram

Uh oh!

Uh oh!

jvsena42 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

piotr-iohk left a comment

Choose a reason for hiding this comment

QA review

Uh oh!

ovi-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ovi-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jvsena42 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

piotr-iohk left a comment

Choose a reason for hiding this comment

QA review

Findings

Uh oh!

Uh oh!

ovi-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

piotr-iohk left a comment

Choose a reason for hiding this comment

QA review

Uh oh!

ovitrif commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

ovi-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jvsena42 left a comment

Choose a reason for hiding this comment

Uh oh!

ovi-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

piotr-iohk left a comment

Choose a reason for hiding this comment

QA review

Uh oh!

jvsena42 left a comment

ben-kaufman commented Sep 24, 2026 •

edited

Loading

github-actions Bot commented Sep 24, 2026 •

edited

Loading

greptile-apps Bot commented Sep 24, 2026 •

edited

Loading

ovitrif commented Sep 28, 2026 •

edited

Loading